Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase arm32/arm64 maximum instruction group size #65153

Merged

Conversation

BruceForstall
Copy link
Member

We require that the maximum number of prolog instructions all fit in one
instruction group. Recent changes appear to have increased the number of
instructions we are generating the prolog, leading to NOWAY assert on
Release builds and test failure on linux-arm64.

Bump up the number to avoid this problem, and leave some headroom for
possible additional needs.

Fixes #64162, #64793.

No SPMI asm diffs, although there are textual asm diffs due to changing the number of
instructions allowed in an instruction group before an "extension" group is created.

We require that the maximum number of prolog instructions all fit in one
instruction group. Recent changes appear to have increased the number of
instructions we are generating the prolog, leading to NOWAY assert on
Release builds and test failure on linux-arm64.

Bump up the number to avoid this problem, and leave some headroom for
possible additional needs.

Fixes dotnet#64162, dotnet#64793.
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 10, 2022
@ghost ghost assigned BruceForstall Feb 10, 2022
@ghost
Copy link

ghost commented Feb 10, 2022

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

We require that the maximum number of prolog instructions all fit in one
instruction group. Recent changes appear to have increased the number of
instructions we are generating the prolog, leading to NOWAY assert on
Release builds and test failure on linux-arm64.

Bump up the number to avoid this problem, and leave some headroom for
possible additional needs.

Fixes #64162, #64793.

No SPMI asm diffs, although there are textual asm diffs due to changing the number of
instructions allowed in an instruction group before an "extension" group is created.

Author: BruceForstall
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@BruceForstall
Copy link
Member Author

@AndyAyersMS @dotnet/jit-contrib PTAL

@BruceForstall
Copy link
Member Author

Hmmmm... the asmdiffs task reported some asmdiffs, even for the run where I saw none. Guess I should investigate why:

https://dev.azure.com/dnceng/public/_build/results?buildId=1605720&view=ms.vss-build-web.run-extensions-tab

@BruceForstall
Copy link
Member Author

One weakness of our asmdiffs pipeline is that it picks a baseline JIT from the JIT rolling build. That means that if your PR is submitted (and kicks off the asmdiffs pipeline) after another JIT change with diffs has merged, but before the rolling build has completed (about 11 minutes for windows x86/x64 -- which is what is used for asmdiffs, plus the delay time to mirror the change to the internal repo), you could baseline against something else. In this case, it looks like the diffs are for SingleAccretions copy prop change #64378, as seen in the "SuperPMI asmdiffs setup (x64)" job:

Invoking: git log --pretty=format:%H 161f13d77d94dbad0e00af95cd10646de3e2b801 -20 -- src/coreclr/jit/*
try 1: 161f13d77d94dbad0e00af95cd10646de3e2b801
try 2: 78143965703ed08a634c7c9a70994debf8ead4e9
Warning: the baseline found is not built with the first git hash with JIT code changes; there may be extraneous diffs

Potentially there could even be "tearing", if the x64 and x86 rolling builds took different amounts of time, and asmdiffs ended up getting different baselines.

A solution would be to build the exact baseline needed as part of the pipeline, but that would be ... work.

@BruceForstall
Copy link
Member Author

/azp run runtime-coreclr superpmi-asmdiffs

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BruceForstall BruceForstall merged commit a01be04 into dotnet:main Feb 11, 2022
@BruceForstall BruceForstall deleted the IncreaseArmInstructionGroupBufferSize branch February 11, 2022 19:10
@am11
Copy link
Member

am11 commented Feb 11, 2022

A solution would be to build the exact baseline needed as part of the pipeline, but that would be ... work.

@BruceForstall, I wrote this workflow: https://github.com/am11/CrossRepoCITesting/blob/master/.github/workflows/jit-diff.yml which finds the nearest parent in dotnet/runtime repo and run the diffs against that branch (also covers the case when JITEEVersionIdentifier was updated). For owner of repo with workflow (CrossRepoCITesting or its fork), this UI is shown:
image
Setting Upload artifacts to 1 publishes this kind of release: https://github.com/am11/CrossRepoCITesting/releases/tag/jit-diffs_1773448298 (I use it sparingly and delete the old/unneeded releases when done inspecting 🙂).

@BruceForstall
Copy link
Member Author

@am11 very nice! We could (should?) do jit-diff (in addition to SuperPMI asmdiffs) in our automated asmdiffs pipeline, or an additional optional pipeline. Your workflow does a build, which is what I was suggesting we might want to do to avoid the delay/tearing problem. We do upload the top diffs, although I'm not sure how easy people find it to use currently.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GitHub_17777 test failing on linux-arm64
3 participants